Add validation for `aws_s3_bucket_metric.name` #25260

justinretzolk · 2022-06-09T17:12:32Z

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Output from acceptance testing:

$ make testacc TESTS=TestAccS3BucketMetric PKG=s3
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/s3/... -v -count 1 -parallel 20 -run='TestAccS3BucketMetric'  -timeout 180m
=== RUN   TestAccS3BucketMetric_basic
=== PAUSE TestAccS3BucketMetric_basic
=== RUN   TestAccS3BucketMetric_withEmptyFilter
=== PAUSE TestAccS3BucketMetric_withEmptyFilter
=== RUN   TestAccS3BucketMetric_withFilterPrefix
=== PAUSE TestAccS3BucketMetric_withFilterPrefix
=== RUN   TestAccS3BucketMetric_withFilterPrefixAndMultipleTags
=== PAUSE TestAccS3BucketMetric_withFilterPrefixAndMultipleTags
=== RUN   TestAccS3BucketMetric_withFilterPrefixAndSingleTag
=== PAUSE TestAccS3BucketMetric_withFilterPrefixAndSingleTag
=== RUN   TestAccS3BucketMetric_withFilterMultipleTags
=== PAUSE TestAccS3BucketMetric_withFilterMultipleTags
=== RUN   TestAccS3BucketMetric_withFilterSingleTag
=== PAUSE TestAccS3BucketMetric_withFilterSingleTag
=== CONT  TestAccS3BucketMetric_basic
=== CONT  TestAccS3BucketMetric_withFilterPrefixAndSingleTag
=== CONT  TestAccS3BucketMetric_withFilterSingleTag
=== CONT  TestAccS3BucketMetric_withFilterPrefixAndMultipleTags
=== CONT  TestAccS3BucketMetric_withFilterMultipleTags
=== CONT  TestAccS3BucketMetric_withFilterPrefix
=== CONT  TestAccS3BucketMetric_withEmptyFilter
--- PASS: TestAccS3BucketMetric_withEmptyFilter (1.75s)
--- PASS: TestAccS3BucketMetric_basic (18.81s)
--- PASS: TestAccS3BucketMetric_withFilterPrefix (29.34s)
--- PASS: TestAccS3BucketMetric_withFilterSingleTag (29.61s)
--- PASS: TestAccS3BucketMetric_withFilterPrefixAndSingleTag (29.69s)
--- PASS: TestAccS3BucketMetric_withFilterPrefixAndMultipleTags (30.06s)
--- PASS: TestAccS3BucketMetric_withFilterMultipleTags (30.43s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/s3 31.345s

Information

As called out in the linked issue, aws_s3_bucket_metric.name must be less than or equal to 64 characters. I've not been able to find any AWS documents that call out this requirement, but tested manually (reproduction notes below) to verify this requirement.

Notes

I have not added any additional testing around this change. This is my first code change for the provider, and in looking around, I wasn't able to find any other areas where there are tests specific to ValidateFuncs. If additional test(s) are needed, I'm happy to write them if someone could point me in the right direction for a reference 🙂

Reproductions proving this behavior

Works:

Configuration:

resource "aws_s3_bucket" "test" {
  bucket = "jretzolk-test"
}

resource "random_string" "test" {
  length  = 64
  special = false
}

resource "aws_s3_bucket_metric" "test" {
  bucket = aws_s3_bucket.test.bucket
  name   = random_string.test.result
}

Output:

Plan: 3 to add, 0 to change, 0 to destroy.
random_string.test: Creating...
random_string.test: Creation complete after 0s [id=KkG9ogxU3duvIhKxIB1fG1WlVkCOqnncU7PjoVTreEzVdlB7oMb99LNDyfgOkmpf]
aws_s3_bucket.test: Creating...
aws_s3_bucket.test: Creation complete after 2s [id=jretzolk-test]
aws_s3_bucket_metric.test: Creating...
aws_s3_bucket_metric.test: Creation complete after 0s [id=jretzolk-test:KkG9ogxU3duvIhKxIB1fG1WlVkCOqnncU7PjoVTreEzVdlB7oMb99LNDyfgOkmpf]

Apply complete! Resources: 3 added, 0 changed, 0 destroyed.

Doesn't work (>64 characters)

Configuration:

resource "aws_s3_bucket" "test" {
  bucket = "jretzolk-test"
}

resource "random_string" "test" {
  length  = 65
  special = false
}

resource "aws_s3_bucket_metric" "test" {
  bucket = aws_s3_bucket.test.bucket
  name   = random_string.test.result
}

Output:

Plan: 3 to add, 0 to change, 0 to destroy.
random_string.test: Creating...
random_string.test: Creation complete after 0s [id=Zs0CBxX27kVb2HNgF2hI5wjLrFi8ioUwiEWuDF7JUrZeSgKZPfxkBzqainal7Pkae]
aws_s3_bucket.test: Creating...
aws_s3_bucket.test: Creation complete after 1s [id=jretzolk-test]
aws_s3_bucket_metric.test: Creating...
╷
│ Error: error putting S3 Bucket Metrics Configuration: MalformedXML: The XML you provided was not well-formed or did not validate against our published schema
│       status code: 400, request id: DH90RA2ZAARJ609B, host id: PL/ee2FJ8tUhy+020Wty95uyStjltfb3K5gnLuNBq/VGGbLs/DKSp7eNc8mR0krLYN8WzjbD5nk=
│
│   with aws_s3_bucket_metric.test,
│   on main.tf line 10, in resource "aws_s3_bucket_metric" "test":
│   10: resource "aws_s3_bucket_metric" "test" {
│
╵

Doesn't work (0 characters)

Configuration:

resource "aws_s3_bucket" "test" {
  bucket = "jretzolk-test"
}

resource "aws_s3_bucket_metric" "test" {
  bucket = aws_s3_bucket.test.bucket
  name   = ""
}

Output:

Plan: 2 to add, 0 to change, 0 to destroy.
aws_s3_bucket.test: Creating...
aws_s3_bucket.test: Creation complete after 2s [id=jretzolk-test]
aws_s3_bucket_metric.test: Creating...
╷
│ Error: error putting S3 Bucket Metrics Configuration: InvalidConfigurationId: The specified configuration id is invalid.
│       status code: 400, request id: HHMMTRDG7C3CVXDZ, host id: 3Icvz8BMuk5eknAFv8PVgi6qpvAPAz08EREixcjfeaZh5I1/OT/p3faE1MUwaMAdUQgjaiVfha4=
│
│   with aws_s3_bucket_metric.test,
│   on main.tf line 12, in resource "aws_s3_bucket_metric" "test":
│   12: resource "aws_s3_bucket_metric" "test" {
│
╵

…tion to be minimum 1 char

anGie44

I cherry-picked @meetreks changes (thanks @meetreks for your contribution as well 🎉 ), with a small update to set the validation minimum to 1 as you had done originally for bucket since validation looked to be added to bucket instead of name by mistake.

Output of acceptance tests:

$ make testacc TESTARGS='-run=TestAccS3BucketMetric_' PKG=s3
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/s3/... -v -count 1 -parallel 20  -run=TestAccS3BucketMetric_ -timeout 180m
=== RUN   TestAccS3BucketMetric_basic
=== PAUSE TestAccS3BucketMetric_basic
=== RUN   TestAccS3BucketMetric_withEmptyFilter
=== PAUSE TestAccS3BucketMetric_withEmptyFilter
=== RUN   TestAccS3BucketMetric_withFilterPrefix
=== PAUSE TestAccS3BucketMetric_withFilterPrefix
=== RUN   TestAccS3BucketMetric_withFilterPrefixAndMultipleTags
=== PAUSE TestAccS3BucketMetric_withFilterPrefixAndMultipleTags
=== RUN   TestAccS3BucketMetric_withFilterPrefixAndSingleTag
=== PAUSE TestAccS3BucketMetric_withFilterPrefixAndSingleTag
=== RUN   TestAccS3BucketMetric_withFilterMultipleTags
=== PAUSE TestAccS3BucketMetric_withFilterMultipleTags
=== RUN   TestAccS3BucketMetric_withFilterSingleTag
=== PAUSE TestAccS3BucketMetric_withFilterSingleTag
=== CONT  TestAccS3BucketMetric_basic
=== CONT  TestAccS3BucketMetric_withFilterPrefixAndSingleTag
=== CONT  TestAccS3BucketMetric_withFilterSingleTag
=== CONT  TestAccS3BucketMetric_withFilterMultipleTags
=== CONT  TestAccS3BucketMetric_withFilterPrefixAndMultipleTags
=== CONT  TestAccS3BucketMetric_withFilterPrefix
=== CONT  TestAccS3BucketMetric_withEmptyFilter
--- PASS: TestAccS3BucketMetric_withEmptyFilter (2.04s)
--- PASS: TestAccS3BucketMetric_basic (23.90s)
--- PASS: TestAccS3BucketMetric_withFilterSingleTag (41.11s)
--- PASS: TestAccS3BucketMetric_withFilterPrefix (41.69s)
--- PASS: TestAccS3BucketMetric_withFilterPrefixAndMultipleTags (41.85s)
--- PASS: TestAccS3BucketMetric_withFilterMultipleTags (41.88s)
--- PASS: TestAccS3BucketMetric_withFilterPrefixAndSingleTag (41.88s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/s3	44.947s

github-actions · 2022-06-23T23:08:56Z

This functionality has been released in v4.20.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

github-actions · 2022-07-24T02:41:40Z

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

Add validation for aws_s3_bucket_metric.name

819549f

Add changelog entry

ed031dd

github-actions bot added documentation Introduces or discusses updates to documentation. service/s3 Issues and PRs that pertain to the s3 service. labels Jun 9, 2022

justinretzolk added enhancement Requests to existing resources that expand the functionality or scope. and removed documentation Introduces or discusses updates to documentation. labels Jun 9, 2022

github-actions bot added documentation Introduces or discusses updates to documentation. and removed enhancement Requests to existing resources that expand the functionality or scope. labels Jun 9, 2022

justinretzolk mentioned this pull request Jun 17, 2022

Initial Commit - for aws-internal-review #25288

Closed

github-actions bot removed the documentation Introduces or discusses updates to documentation. label Jun 21, 2022

Initial Commit - fo aws-internal-review

ebcb971

anGie44 force-pushed the b-aws_s3_bucket_metric-name-validation branch from 8fc422f to b88219c Compare June 21, 2022 17:15

github-actions bot added the documentation Introduces or discusses updates to documentation. label Jun 21, 2022

r/s3_bucket_metric: remove validation from bucket; update name valida…

2b1449f

…tion to be minimum 1 char

anGie44 force-pushed the b-aws_s3_bucket_metric-name-validation branch from b88219c to 2b1449f Compare June 21, 2022 17:16

anGie44 approved these changes Jun 21, 2022

View reviewed changes

anGie44 added the enhancement Requests to existing resources that expand the functionality or scope. label Jun 21, 2022

anGie44 added this to the v4.20.0 milestone Jun 21, 2022

anGie44 self-assigned this Jun 21, 2022

anGie44 merged commit fb3807d into main Jun 21, 2022

anGie44 deleted the b-aws_s3_bucket_metric-name-validation branch June 21, 2022 17:55

github-actions bot pushed a commit that referenced this pull request Jun 21, 2022

Update CHANGELOG.md for #25260

164cb53

github-actions bot locked as resolved and limited conversation to collaborators Jul 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add validation for `aws_s3_bucket_metric.name` #25260

Add validation for `aws_s3_bucket_metric.name` #25260

justinretzolk commented Jun 9, 2022

anGie44 left a comment •

edited

Loading

github-actions bot commented Jun 23, 2022

github-actions bot commented Jul 24, 2022

Add validation for aws_s3_bucket_metric.name #25260

Add validation for aws_s3_bucket_metric.name #25260

Conversation

justinretzolk commented Jun 9, 2022

Community Note

Information

Notes

Reproductions proving this behavior

Works:

Doesn't work (>64 characters)

Doesn't work (0 characters)

anGie44 left a comment • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Jun 23, 2022

github-actions bot commented Jul 24, 2022

Add validation for `aws_s3_bucket_metric.name` #25260

Add validation for `aws_s3_bucket_metric.name` #25260

anGie44 left a comment •

edited

Loading